signalTracker: Explicitly register destroyable types

We currently assume that any '::destroy' signal on a GObject type
has the semantics of the ClutterActor/GtkWidget signal, and should
therefore result in all signals being disconnected.

But we already have a case where the assumption doesn't hold: ShellWM
uses '::destroy' for the closing animation of windows, and the ShellWM
object itself remains very valid after the emission.

So rather than making assumptions about '::destroy', check objects
against a list of destroyable types that are explicitly registered
as such.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2226>

diff --git a/js/misc/signalTracker.js b/js/misc/signalTracker.js
index 3e83be73411db3d864e20bea065b03fc48e9498f..bcfa72de977835c0e27ec8bce493bb61a9daf00a 100644 (file)
--- a/js/misc/signalTracker.js
+++ b/js/misc/signalTracker.js
@@ -1,14 +1,15 @@
 /* exported TransientSignalHolder, addObjectSignalMethods */
 const { GObject } = imports.gi;
 
+const destroyableTypes = [];
+
 /**
  * @private
  * @param {Object} obj - an object
  * @returns {bool} - true if obj has a 'destroy' GObject signal
  */
 function _hasDestroySignal(obj) {
-    return obj instanceof GObject.Object &&
-        GObject.signal_lookup('destroy', obj);
+    return destroyableTypes.some(type => obj instanceof type);
 }
 
 var TransientSignalHolder = GObject.registerClass(
@@ -28,6 +29,7 @@ class TransientSignalHolder extends GObject.Object {
         this.emit('destroy');
     }
 });
+registerDestroyableType(TransientSignalHolder);
 
 class SignalManager {
     /**
@@ -230,3 +232,19 @@ function addObjectSignalMethods(proto) {
     };
     proto['disconnect_object'] = proto['disconnectObject'];
 }
+
+/**
+ * Register a GObject type as having a 'destroy' signal
+ * that should disconnect all handlers
+ *
+ * @param {GObject.Type} gtype - a GObject type
+ */
+function registerDestroyableType(gtype) {
+    if (!GObject.type_is_a(gtype, GObject.Object))
+        throw new Error(`${gtype} is not a GObject subclass`);
+
+    if (!GObject.signal_lookup('destroy', gtype))
+        throw new Error(`${gtype} does not have a destroy signal`);
+
+    destroyableTypes.push(gtype);
+}

diff --git a/js/ui/environment.js b/js/ui/environment.js
index 2d3baf72254a095adef2243cc92452ca7935b2ce..7bd848f1ab2509ba257fe0fe4e6f6a9f8261ec9c 100644 (file)
--- a/js/ui/environment.js
+++ b/js/ui/environment.js
@@ -345,6 +345,8 @@ function init() {
         SignalTracker.addObjectSignalMethods(prototype);
     };
 
+    SignalTracker.registerDestroyableType(Clutter.Actor);
+
     // Miscellaneous monkeypatching
     _patchContainerClass(St.BoxLayout);
 

diff --git a/js/ui/messageTray.js b/js/ui/messageTray.js
index 862ade6ef20709b4a3232d9c04cfd0c72cb0a326..e001f2202b2ec5334c28f67b9afbb5cb3e4944b1 100644 (file)
--- a/js/ui/messageTray.js
+++ b/js/ui/messageTray.js
@@ -10,6 +10,7 @@ const GnomeSession = imports.misc.gnomeSession;
 const Layout = imports.ui.layout;
 const Main = imports.ui.main;
 const Params = imports.misc.params;
+const SignalTracker = imports.misc.signalTracker;
 
 const SHELL_KEYBINDINGS_SCHEMA = 'org.gnome.shell.keybindings';
 
@@ -493,6 +494,7 @@ var Notification = GObject.registerClass({
         this.run_dispose();
     }
 });
+SignalTracker.registerDestroyableType(Notification);
 
 var NotificationBanner = GObject.registerClass({
     Signals: {
@@ -795,6 +797,7 @@ var Source = GObject.registerClass({
         }
     }
 });
+SignalTracker.registerDestroyableType(Source);
 
 var MessageTray = GObject.registerClass({
     Signals: {

diff --git a/tests/unit/signalTracker.js b/tests/unit/signalTracker.js
index 40398c0b8a588a4cd0f8cccb00d2c7fa06bba0b2..a6f50e257b0595d24f722d9840b689a028fbf490 100644 (file)
--- a/tests/unit/signalTracker.js
+++ b/tests/unit/signalTracker.js
@@ -8,13 +8,14 @@ const JsUnit = imports.jsUnit;
 const Signals = imports.signals;
 
 const Environment = imports.ui.environment;
-const { TransientSignalHolder } = imports.misc.signalTracker;
+const { TransientSignalHolder, registerDestroyableType } = imports.misc.signalTracker;
 
 Environment.init();
 
 const Destroyable = GObject.registerClass({
     Signals: { 'destroy': {} },
 }, class Destroyable extends GObject.Object {});
+registerDestroyableType(Destroyable);
 
 class PlainEmitter {}
 Signals.addSignalMethods(PlainEmitter.prototype);